Improve k8s canonical DNS lookup#138
Merged
astrojuanlu merged 1 commit intocanonical:8.0/edgefrom Mar 11, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0/edge #138 +/- ##
===========================================
Coverage ? 59.91%
===========================================
Files ? 34
Lines ? 6364
Branches ? 1069
===========================================
Hits ? 3813
Misses ? 2233
Partials ? 318 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gboutry
force-pushed
the
fix/fqdn-resolution
branch
from
d67481a to
3112e57
Compare
Replace getfqdn-based k8s hostname resolution with a canonical-name lookup built on getaddrinfo(AI_CANONNAME). Handle resolver failures, keep the trailing dot returned by DNS, and update the Kubernetes unit tests for the charm, provider, upgrade, and utility helpers. Fixes canonical#137 Signed-off-by: Guillaume Boutry <guillaume.boutry@canonical.com>
gboutry
force-pushed
the
fix/fqdn-resolution
branch
from
3112e57 to
647d6c1
Compare
Contributor
|
Thank you @gboutry . We'll nudge the CI so that the s390x builds are retried and the integration tests are triggered. I see you added new unit tests 👍🏼 Any chance to validate whether this PR fixes the original issue? |
sinclert-canonical
approved these changes
Mar 10, 2026
astrojuanlu
approved these changes
Mar 10, 2026
Contributor
|
Backups and subordinate tests are failing, as expected (see #119 (comment)) I will retry the 2 remaining tests a few more times just in case. |
Contributor
|
Been retrying the last test a few times. Despite not passing, the timeouts seem to be happening in different places, and there's nothing in the logs that suggests that they're caused by this PR. So I'm going to go ahead and merge. Thanks again @gboutry ! |
astrojuanlu
pushed a commit
that referenced
this pull request
Mar 17, 2026
Replace getfqdn-based k8s hostname resolution with a canonical-name lookup built on getaddrinfo(AI_CANONNAME). Handle resolver failures, keep the trailing dot returned by DNS, and update the Kubernetes unit tests for the charm, provider, upgrade, and utility helpers. Fixes #137 Signed-off-by: Guillaume Boutry <guillaume.boutry@canonical.com>
2 tasks
astrojuanlu
pushed a commit
that referenced
this pull request
Mar 17, 2026
Replace getfqdn-based k8s hostname resolution with a canonical-name lookup built on getaddrinfo(AI_CANONNAME). Handle resolver failures, keep the trailing dot returned by DNS, and update the Kubernetes unit tests for the charm, provider, upgrade, and utility helpers. Fixes #137 Signed-off-by: Guillaume Boutry <guillaume.boutry@canonical.com>
astrojuanlu
added a commit
that referenced
this pull request
Mar 17, 2026
Replace getfqdn-based k8s hostname resolution with a canonical-name lookup built on getaddrinfo(AI_CANONNAME). Handle resolver failures, keep the trailing dot returned by DNS, and update the Kubernetes unit tests for the charm, provider, upgrade, and utility helpers. Fixes #137 Signed-off-by: Guillaume Boutry <guillaume.boutry@canonical.com> Co-authored-by: Guillaume Boutry <boutryguillaume1@gmail.com>
delgod
reviewed
Mar 18, 2026
Comment on lines
+101
to
+103
| for entry in info: | ||
| if canonname := entry[3]: | ||
| return canonname |
Member
There was a problem hiding this comment.
I don't sure that logic is robust enough with different DNS engines for kubernetes.
Contributor
Member
There was a problem hiding this comment.
I think we should add the following function
import re
def is_srv_dns_record(s: str) -> bool:
"""Match pattern: pod-name.service-name.srv.some.domain.name."""
pattern = r'^[a-z0-9]([a-z0-9-]*[a-z0-9])?\.([a-z0-9]([a-z0-9-]*[a-z0-9])?\.)+srv(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?){2,}\.?$'
return bool(re.match(pattern, s, re.IGNORECASE))
and use it here
Suggested change
| for entry in info: | |
| if canonname := entry[3]: | |
| return canonname | |
| for entry in info: | |
| if canonname := entry[3] and is_srv_dns_record(canonname): | |
| return canonname |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Fixes #137.
Solution
Replace
getfqdn-based Kubernetes hostname resolution with a canonical-namelookup built on
socket.getaddrinfo(..., AI_CANONNAME).The helper now scans resolver results for a canonical name, handles resolver
failures explicitly, preserves the trailing dot returned by DNS, and updates
the unit tests covering the charm, provider, upgrade, and utility paths.
Checklist